Update comment to specify minivan TYPE_CAR categorization - #910
johschmitz wants to merge 1 commit into
Conversation
|
Shouldn't it be clear already which category to use because the other car classifications are deprecated and are not recommended to be used? |
|
@thomassedlmayer maybe for someone deep in the trenches of OSI this is clear. For someone like me who just has very occasional contact it is not clear. My expectation as a software engineer though would be that it is written as clear as possible for a broad audience. Also my expectation nowadays as an LLM user would be to make APIs and their documentations simple and explicit. It would take an LLM agent significantly more tokens to find out this whole depreciation based constraints and implement it right compared to just writing this explicitly like I am proposing. |
|
but it does not hurt at all to make this small change? |
|
Trying to recap our discussion we had in at online meeting in the coordination group before holidays: All comments are directly copied from the ASAM TrafficParticipants specification and we should not alter the comments here in OSI but instead change the specification itself if there is a need. The CCB suggests to close this PullRequest. |
|
Makes sense to me to copy the text a directly as possible but doesn't make sense to me to not allow to interlink the documentation in a project specific way that fully respects the intention of the traffic participant spec. How about moving this into a sentence in a note block instead? Apparently note blocks are "allowed" since you are using them in other types as well? |
|
@johschmitz I agree that having The mapping of cars in OSI is extensively discussed in the VehicleClassification::Type description. That's why I said before that I think it should be sufficiently clear from the documentation which category to use: the former car-specific enums are deprecated in favor of TYPE_CAR, which represents the recommended mapping for cars. This is also reflected in the individual descriptions of the deprecated types. In my opinion the documentation consistently points users toward TYPE_CAR even if in that single case the documentation only mentions "cars". In that sense, all paths through the documentation lead to TYPE_CAR. Another note saying that "car" maps to In my opinion the better solution would be to discuss a designated way to insert standard-specific enums/names in harmonized descriptions and also apply this to other standards accordingly. This could certainly be discussed in the next harmonization project (https://code.asam.net/simulation/harmonization/trafficparticipants/traffic-participants-specification/-/issues). Maybe it's useful in other instances too. More importantly,having looked at both OSI documentation and Traffic Participants Specification again, I noticed that the sentence was actually moved into the main/normative description block in the harmonized spec after the OSI adaptations have been merged. This is actually something that should be adapted.
|
Signed-off-by: Johannes Schmitz <johannes.schmitz1@gmail.com>
|
Updated my commit, maybe this could satisfy all stakeholders now? Otherwise please suggest edits that would lead towards that point. Note: I corrected mini vans to minivans since I believe that is the correct english spelling (see https://en.wikipedia.org/wiki/Minivan, https://www.merriam-webster.com/dictionary/minivan). But if typos need to be copied over from the original text as well I can reintroduce it. |


Reference to a related issue in the repository
#909
Add a description
Make description unambiguous.
Some questions to ask:
What is this change?
What does it fix?
Is this a bug fix or a feature? Does it break any existing functionality or force me to update to a new version?
How has it been tested?
Take this checklist as orientation for yourself, if this PR is ready for the Change Control Board:
If you can’t check all of them, please explain why.
If all boxes are checked or commented and you have achieved at least one positive review, you can assign the label ReadyForCCBReview!